Editor: Stop :has() selectors recalculating the whole document on every block selection - #81471
Conversation
…very block selection
github-actions
Bot
added
[Package] Editor
|
Size Change: +80 B (0%) Total Size: 7.71 MB 📦 View Changed
|
I think we should remove this throwaway case, as it can mask an actual regression. For example, the editable root change (removed in #81184) affected the first selection significantly, but switching between blocks was cheaper. So, as a user opening a large post, the first selection was junky, but this was missed by our specs. The downside is that our metrics will probably jump, but they'll show numbers close to the actual median. cc @ellatrix, @youknowriad, @tyxla |
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Here are the metrics from the last two CI runs. Screenshots
|
|
Really cool. I didn't know a CSS change could have such a huge impact on our metrics. Good job. On the ignoring of the first selection, I wonder if it means we should have two metrics, first selection, consecutive ones. I guess my main concern is whether we're computing an average that doesn't make sense. |
|
I need to check the Git history and confirm if adding a throwaway step was intentional or just something copied and pasted from an existing spec. Maybe we should measure the average of the first block selection (clear selection between iterations). This is usually when regressions are noticeable, since controls need to be mounted, etc. Consecutive ones or switching between blocks is also interesting, but here we have more variables. Switching between the same block types can be faster than switching from Paragraph to Navigation or any other block with complex controls. |
|
I think I added it intentionally but that was a long time ago haha |
Yes I like that. |
|
I'll follow up on that separately, also curious if @ellatrix has any suggestions. Meanwhile, should we merge this? |
| // in the editor recalculates the whole document. | ||
| &:has(.editor-editor-interface.is-distraction-free) { | ||
| --wp-admin--admin-bar--height: 0px; | ||
| --wp-editor-admin-bar-display: none; |
There was a problem hiding this comment.
I know some of this is pre-existent, But I'd prefer that we avoid CSS variables as much as possible as they become kind of APIs. Might not be always possible though.
|
I'm seeing an issue but it might not be specific to this PR. I have "show template" enabled, I go into "distraction free" and when I click "styles" in the more menu, nothing happens, the styles sidebar doesn't show up and there's also a double scrollbar I think. |
youknowriad
left a comment
youknowriad
left a comment
There was a problem hiding this comment.
This looks good to me (need to confirm whether that previous bug is specific to this branch or not)
|
I can reproduce the |
…very block selection (#81471) Co-authored-by: Mamaduka <mamaduka@git.wordpress.org> Co-authored-by: youknowriad <youknowriad@git.wordpress.org>
|
Found this from checking in on the code vitals. I skipped over this commit initially because I didn't think a |
|
Was reading this X thread recently, which is also an interesting case. Claude also makes writing perf probes a lot better. |
3 participants
What?
Related #81457.
Rewrites four
:has()rules in the editor and interface styles so that each:has()compound is the subject of its own rule.Why?
Clicking a paragraph in a 1000-paragraph post ran about 8 full document-style recalcs in the
post.phpframe. The cause isbody:has(.editor-editor-interface.is-distraction-free) #wpadminbar { display: none }. Blink treats a:has()whose subject sits below the anchor differently from one where the anchor is the subject: it cannot tell which descendants are affected, so it flags the anchor's entire subtree and recalculates it on every DOM change during the React commit. The same pattern appeared in the revisions timeline and the collab sidebar rules.Splitting the distraction-free rule in two confirmed where the cost was. The inherited custom property half is free, and the
#wpadminbarhalf accounts for all of it.How?
The distraction-free state now reaches the admin bar through a second inherited custom property, so
bodystays the subject of its own:has(). The revisions timeline rules are unnested, since their subjects exist only in revisions mode and do not require the extra scoping. The collab sidebar header is targeted by its own class instead of a:has()on the skeleton sidebar.Results
Measured across two real builds with the arms alternating within one session, since sequential A/B blocks are not reliable at this size.
Selecting blocksperf metric (median of 40 clicks)BODYsubtree invalidations per clickThe document has 792 elements, so the whole document recalcs are gone.
The
Selecting blocksmetric discards the first click, so it reports the steady state improvement of about 17 percent rather than the larger first click one. Head won all four rounds with no overlap between the per-round ranges.Testing Instructions
Some of these have e2e test coverage, so the smoke test affected features:
Testing Instructions for Keyboard
Same.
Use of AI Tools
Assisted by Claude